Skip to content

PARAF-503: Added acroform signature tags validation - #45

Open
chris-adam wants to merge 1 commit into
mainfrom
PARAF-503/acroform-validation
Open

PARAF-503: Added acroform signature tags validation#45
chris-adam wants to merge 1 commit into
mainfrom
PARAF-503/acroform-validation

Conversation

@chris-adam

@chris-adam chris-adam commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

A voir aussi comment on veut formuler les messages d'erreur. J'attends votre avis

  • Check les balises pour l'acroform pour les signataires et le sceau électronique (soit aucun tag, soit un tag par signataire ou sceau)
  • Si la validation ne passe pas: affiche un message sur le container et le fichier
  • Lors de la création de la session externe, l'envoi est bloqué avec un message d'erreur
  • Plus aucune validation si la session n'est plus en draft (en création)
  • Ajout d'un message pour informer des balises acroform détectées lors de l'upload d'une annexe

Summary by CodeRabbit

  • New Features

    • Added validation for signature and seal tags in PDF, ODT, and DOCX documents.
    • Displays detected tags when files are added.
    • Shows translated errors for missing, duplicate, unknown, or mismatched tags.
    • Prevents sending sessions when document tags do not match configured signers or seals.
  • Documentation

    • Added a changelog entry for AcroForm tag validation.
  • Tests

    • Added comprehensive coverage for document parsing, validation, error reporting, and event notifications.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bcc0b586-11d3-4799-be4a-5fe1a532f5c7

📥 Commits

Reviewing files that changed from the base of the PR and between f0f5fc4 and 0000cb4.

📒 Files selected for processing (2)
  • src/imio/esign/events.py
  • src/imio/esign/tests/test_events.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/imio/esign/events.py
  • src/imio/esign/tests/test_events.py

📝 Walkthrough

Walkthrough

The change adds AcroForm tag extraction and validation for PDF, ODT, and DOCX files. It blocks invalid external sessions, displays draft-session errors, reports tags when annexes are added, and adds supporting tests and dependencies.

Changes

AcroForm validation flow

Layer / File(s) Summary
Tag extraction and validation
src/imio/esign/acroform.py, src/imio/esign/tests/*, setup.py, .github/workflows/main.yml, test-4.3.cfg, CHANGES.rst
The new module extracts and validates signer and seal tags. Tests cover supported files, malformed inputs, session errors, and formatted messages. Dependencies and CI packages support the test suite.
Session blocking and browser errors
src/imio/esign/browser/..., src/imio/esign/tests/test_browser_views.py
External session creation stops when validation fails. A draft-session viewlet renders translated errors for affected annexes and containers.
Annex tag notification
src/imio/esign/events.py, src/imio/esign/events.zcml, src/imio/esign/tests/test_events.py
An annex-added subscriber reports detected tags and ignores generated files or files without tags.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ExternalSessionCreateView
  participant get_session_acroform_errors
  participant create_external_session
  User->>ExternalSessionCreateView: submit session
  ExternalSessionCreateView->>get_session_acroform_errors: validate AcroForm tags
  alt validation errors
    get_session_acroform_errors-->>ExternalSessionCreateView: return file errors
    ExternalSessionCreateView-->>User: show translated errors
  else validation succeeds
    ExternalSessionCreateView->>create_external_session: create session
    create_external_session-->>User: redirect to external service
  end
Loading

Possibly related PRs

Suggested reviewers: sgeulette

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: validation of AcroForm signature tags.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch PARAF-503/acroform-validation

Comment @coderabbitai help to get the list of available commands.

@chris-adam
chris-adam force-pushed the PARAF-503/acroform-validation branch 4 times, most recently from a1b0954 to 823161e Compare August 12, 2026 07:41
@chris-adam
chris-adam marked this pull request as ready for review August 12, 2026 07:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (1)
src/imio/esign/tests/test_acroform.py (1)

36-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add DOCX extraction coverage.

extract_text() supports DOCX through word/document.xml, but test_extract_text() covers only PDF and ODT. Add a DOCX fixture with signature and seal tags, plus a corrupt DOCX case.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/tests/test_acroform.py` around lines 36 - 68, Add DOCX
coverage to test_extract_text by creating a DOCX fixture containing signature
and seal tags, asserting extract_text returns the expected tags and text, and
adding a corrupt DOCX input assertion that returns an empty string. Reuse the
existing tag helpers and NamedBlobFile/content-type conventions used by the PDF
and ODT cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/imio/esign/acroform.py`:
- Around line 61-66: Update the archive-reading flow around
zip_file.read(part_name) to retrieve the member’s ZipInfo and reject content.xml
when its file_size exceeds the application’s configured decompression limit
before reading it. Preserve the existing safe_unicode, XML processing, and
cleanup behavior for members within the limit.
- Around line 98-102: Update the signer-ID handling around SIGNER_ID_RE and the
numbers append logic to inspect the raw captured identifier before converting it
with int(). Reject identifiers with leading zeros, such as Signer01, while
continuing to accept canonical forms like Signer1 and preserving the existing
seal handling.
- Around line 80-85: Update the text-extraction flow around the OSError and
generic Exception handlers so failures return an explicit extraction error
rather than empty text. Update check_file to recognize and reject that error,
ensuring ExternalSessionCreateView does not create a session until PDF
extraction succeeds; preserve normal tag validation for successful extraction.
- Around line 43-49: Update the subprocess flow around Popen and communicate to
enforce a Python 2.7-compatible execution timeout without using
communicate(timeout=...). On expiry, terminate or kill the pdftotext process,
reap it with wait/communicate, and handle the extraction failure consistently
with the surrounding API, including stderr details where appropriate.

In `@src/imio/esign/browser/configure.zcml`:
- Around line 91-99: Update the esign-acroform-errors browser:viewlet
registration to target the applicable folder interface in addition to the annex
content interface, so folder0 containers resolve the viewlet. Add an integration
test that performs a ZCML/browser viewlet lookup for the folder interface and
verifies the registration is available.

In `@src/imio/esign/browser/templates/acroform_errors.pt`:
- Around line 10-13: Update the message loop in the acroform error template so
each validation message rendered by the item[1] repeat has a clear separator,
such as a line break or nested list item. Preserve the existing file title link
and message content while ensuring duplicate and missing-tag errors render as
distinct readable entries.

In `@src/imio/esign/browser/views.py`:
- Around line 172-188: Gate the get_session_acroform_errors validation and its
error response in the surrounding endpoint so they run only when the session is
in a draft state. Preserve the existing blocking behavior for draft sessions,
and add coverage confirming that a non-draft session bypasses validation and
proceeds normally.

---

Nitpick comments:
In `@src/imio/esign/tests/test_acroform.py`:
- Around line 36-68: Add DOCX coverage to test_extract_text by creating a DOCX
fixture containing signature and seal tags, asserting extract_text returns the
expected tags and text, and adding a corrupt DOCX input assertion that returns
an empty string. Reuse the existing tag helpers and NamedBlobFile/content-type
conventions used by the PDF and ODT cases.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca07585a-5801-44b3-bd7e-e387fddd44cc

📥 Commits

Reviewing files that changed from the base of the PR and between 64c4a53 and 823161e.

⛔ Files ignored due to path filters (1)
  • src/imio/esign/tests/signer_tags.pdf is excluded by !**/*.pdf
📒 Files selected for processing (14)
  • .github/workflows/main.yml
  • CHANGES.rst
  • setup.py
  • src/imio/esign/acroform.py
  • src/imio/esign/browser/configure.zcml
  • src/imio/esign/browser/templates/acroform_errors.pt
  • src/imio/esign/browser/views.py
  • src/imio/esign/events.py
  • src/imio/esign/events.zcml
  • src/imio/esign/tests/base.py
  • src/imio/esign/tests/test_acroform.py
  • src/imio/esign/tests/test_browser_views.py
  • src/imio/esign/tests/test_events.py
  • test-4.3.cfg

Comment on lines +43 to +49
process = subprocess.Popen(
[PDFTOTEXT, "-q", "-enc", "UTF-8", "-", "-"],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
out, err = process.communicate(data)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect declared Python targets before using Popen.communicate(timeout=...).
fd -HI -t f . |
  rg '(^|/)(pyproject\.toml|setup\.py|setup\.cfg|tox\.ini|\.python-version|\.tool-versions|Dockerfile|.*\.ya?ml)$' |
  xargs -r rg -n -i -C2 'python(_requires|-version)?|image:'

Repository: IMIO/imio.esign

Length of output: 4129


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- source outline ---'
ast-grep outline src/imio/esign/acroform.py

printf '%s\n' '--- relevant source ---'
cat -n src/imio/esign/acroform.py | sed -n '1,130p'

printf '%s\n' '--- package metadata and usages ---'
cat -n setup.py | sed -n '1,90p'
rg -n -C3 'extract_text|PDFTOTEXT|communicate\(|Popen\(' . -g '*.py' -g '!*.pyc'

Repository: IMIO/imio.esign

Length of output: 17037


Bound pdftotext execution time.

The CI matrix runs Python 2.7, so Popen.communicate(timeout=...) is unavailable. Implement a Python 2.7-compatible timeout, kill and reap the process on expiry, and handle the extraction failure.

🧰 Tools
🪛 Ruff (0.16.1)

[error] 43-43: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/acroform.py` around lines 43 - 49, Update the subprocess flow
around Popen and communicate to enforce a Python 2.7-compatible execution
timeout without using communicate(timeout=...). On expiry, terminate or kill the
pdftotext process, reap it with wait/communicate, and handle the extraction
failure consistently with the surrounding API, including stderr details where
appropriate.

Comment on lines +61 to +66
zip_file = zipfile.ZipFile(BytesIO(data))
try:
xml = safe_unicode(zip_file.read(part_name))
finally:
zip_file.close()
return unescape(XML_TAG_RE.sub(u" ", xml), {u""": u'"', u"'": u"'"})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Bound decompression of content.xml.

ZipFile.read(part_name) expands the complete archive member in memory. An uploaded DOCX or ODT can declare a very large uncompressed XML part and exhaust a request worker. Check ZipInfo.file_size against an application limit before reading the member.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/acroform.py` around lines 61 - 66, Update the archive-reading
flow around zip_file.read(part_name) to retrieve the member’s ZipInfo and reject
content.xml when its file_size exceeds the application’s configured
decompression limit before reading it. Preserve the existing safe_unicode, XML
processing, and cleanup behavior for members within the limit.

Comment on lines +80 to +85
except OSError:
logger.error("Cannot run '%s': the signature tags of pdf files are not checked!", PDFTOTEXT)
return u""
except Exception:
logger.debug("Could not extract the text of file '%s'", nbf.filename, exc_info=True)
return u""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not treat extraction failures as files without tags.

These handlers return empty text for corrupt files, failed conversions, and an unavailable pdftotext binary. check_file then treats the file as valid, so ExternalSessionCreateView sends a session without validation. Return an explicit extraction error and block the session until extraction succeeds.

🧰 Tools
🪛 Ruff (0.16.1)

[warning] 83-83: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/acroform.py` around lines 80 - 85, Update the text-extraction
flow around the OSError and generic Exception handlers so failures return an
explicit extraction error rather than empty text. Update check_file to recognize
and reject that error, ensuring ExternalSessionCreateView does not create a
session until PDF extraction succeeds; preserve normal tag validation for
successful extraction.

Comment on lines +98 to +102
match = SIGNER_ID_RE.search(payload)
if match:
numbers.append(int(match.group(1)))
elif SEAL_ID_RE.search(payload):
seals += 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject noncanonical signer identifiers.

int(match.group(1)) converts Signer01 to signer 1. The validator then accepts a tag that is not the documented Signer1 identifier. Preserve the raw identifier and reject leading-zero forms before validation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/acroform.py` around lines 98 - 102, Update the signer-ID
handling around SIGNER_ID_RE and the numbers append logic to inspect the raw
captured identifier before converting it with int(). Reject identifiers with
leading zeros, such as Signer01, while continuing to accept canonical forms like
Signer1 and preserving the existing seal handling.

Comment on lines +91 to +99
<browser:viewlet
zcml:condition="installed imio.annex"
for="imio.annex.content.annex.IAnnex"
manager="plone.app.layout.viewlets.interfaces.IAboveContentTitle"
name="esign-acroform-errors"
template="templates/acroform_errors.pt"
class=".views.AcroformErrorsViewlet"
permission="zope2.View"
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Identify the interfaces and portal types used by folder0 and annex0.
ast-grep outline src/imio/esign/tests/base.py --items all
rg -n -C3 'folder0|annex0|IAnnex|portal_types' \
  src/imio/esign/tests/base.py \
  src/imio/esign/tests/test_browser_views.py

# Confirm the only registration scope for the new viewlet.
rg -n -C5 'esign-acroform-errors|AcroformErrorsViewlet|IAnnex' \
  src/imio/esign/browser/configure.zcml

Repository: IMIO/imio.esign

Length of output: 11906


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- viewlet implementation and tests ---'
rg -n -C8 'class AcroformErrorsViewlet|AcroformErrorsViewlet|IAnnex|folder0|annex0' \
  src/imio/esign/browser src/imio/esign/tests

printf '%s\n' '--- test fixture and dependency declarations ---'
rg -n -C6 'folder0|annex0|imio\.annex|IAnnex|content type|portal_type' \
  src/imio/esign \
  setup.py setup.cfg pyproject.toml buildout.cfg versions.cfg tox.ini 2>/dev/null || true

printf '%s\n' '--- candidate local interfaces and registrations ---'
rg -n -C5 'I[A-Za-z]*(Folder|Container|Annex)|providedBy|adapted|viewlet' \
  src/imio/esign

Repository: IMIO/imio.esign

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- viewlet implementation ---'
sed -n '319,390p' src/imio/esign/browser/views.py

printf '%s\n' '--- focused viewlet test ---'
sed -n '650,698p' src/imio/esign/tests/test_browser_views.py

printf '%s\n' '--- test layer and fixture setup ---'
sed -n '1,180p' src/imio/esign/tests/base.py
sed -n '1,220p' src/imio/esign/testing.py

printf '%s\n' '--- dependency declarations ---'
rg -n -C4 'imio\.annex|install_requires|eggs|extends|test_suite' \
  setup.py setup.cfg pyproject.toml buildout.cfg tox.ini versions.cfg \
  src/imio/esign 2>/dev/null || true

printf '%s\n' '--- repository files that define fixture content ---'
git ls-files | rg '(^|/)(testing|fixtures?|profiles|default|setuphandlers|.*\.zcml$)|annex'

Repository: IMIO/imio.esign

Length of output: 17500


🌐 Web query:

IMIO imio.annex GitHub IAnnex Folder interface

💡 Result:

imio.annex is a Python package and add-on for the Plone content management system, developed by the Intercommunale de Mutualisation Informatique et Organisationnelle (IMIO) [1][2]. It is primarily used in conjunction with other IMIO tools, such as Products.PloneMeeting, to manage document annexes [3]. The code for the project is hosted in the collective GitHub organization under the repository path collective/imio.annex [4][1]. While there is an IMIO/imio.annex repository on GitHub [4], its README points to the collective/imio.annex repository as the official source for the code [4]. Regarding the IAnnex interface, this is a marker interface or contract defined within the imio.annex package [3]. It is used to identify and interact with annex objects within the Plone framework [3]. Development documentation for such packages typically follows standard Plone and Zope interface patterns, where interfaces are defined in an interfaces.py file within the package. Key details: - Project: imio.annex [1] - Source Code: https://github.com/collective/imio.annex [4][1] - Current Status: Mature, actively maintained (latest version 2.27.1 as of January 2026) [1][5][6] - Ecosystem: Integrated within the IMIO software suite for public administration (often used with Products.PloneMeeting) [3][7]

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for repo in IMIO/imio.annex collective/imio.annex; do
  printf '%s\n' "--- $repo ---"
  curl -fsSL "https://api.github.com/repos/$repo" |
    jq -r '[.full_name, .default_branch] | `@tsv`'
done

repo=collective/imio.annex
branch=$(curl -fsSL "https://api.github.com/repos/$repo" | jq -r '.default_branch')
curl -fsSL "https://api.github.com/repos/$repo/git/trees/$branch?recursive=1" |
  jq -r '.tree[] | select(.path | test("(^|/)(interfaces|annex|testing|configure).*\\.py$|\\.zcml$")) | .path' |
  head -80

Repository: IMIO/imio.esign

Length of output: 281


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo=IMIO/imio.annex
branch=master

curl -fsSL "https://api.github.com/repos/$repo/git/trees/$branch?recursive=1" |
  jq -r '.tree[] | select(.path | test("(^|/)(interfaces|annex|testing|configure).*\\.py$|\\.zcml$")) | .path' |
  head -100

Repository: IMIO/imio.esign

Length of output: 950


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base=https://raw.githubusercontent.com/IMIO/imio.annex/master/src/imio/annex

printf '%s\n' '--- interfaces.py ---'
curl -fsSL "$base/interfaces.py" | sed -n '1,220p'

printf '%s\n' '--- content/annex.py ---'
curl -fsSL "$base/content/annex.py" | sed -n '1,220p'

printf '%s\n' '--- content/configure.zcml ---'
curl -fsSL "$base/content/configure.zcml" | sed -n '1,220p'

Repository: IMIO/imio.esign

Length of output: 4256


Register the viewlet for the folder interface.

folder0 is a Folder, while IAnnex is implemented by annex objects only. The direct viewlet test does not exercise ZCML lookup, so container errors will not render. Add a viewlet registration for the applicable folder interface and an integration lookup test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/browser/configure.zcml` around lines 91 - 99, Update the
esign-acroform-errors browser:viewlet registration to target the applicable
folder interface in addition to the annex content interface, so folder0
containers resolve the viewlet. Add an integration test that performs a
ZCML/browser viewlet lookup for the folder interface and verifies the
registration is available.

Comment thread src/imio/esign/browser/templates/acroform_errors.pt Outdated
Comment on lines +172 to +188
acroform_errors = get_session_acroform_errors(int(session_id))
if acroform_errors:
audit("send_to_external_service", "session={} error=acroform".format(session_id))
api.portal.show_message(
_(
"Session ${id} not sent because signature or seal tags are wrong: ${details}",
mapping={
"id": session_id,
"details": format_errors(
[(obj, messages) for obj, _cuid, messages in acroform_errors.values()], self.request
),
},
),
request=self.request,
type="error",
)
return self.context.absolute_url() + "/@@parapheo"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Skip validation after the session leaves a draft state.

This path calls get_session_acroform_errors for every session state. That utility has no state filter. An invalid file can therefore block this endpoint after the session is sent, although the PR requires validation to stop after draft status. Gate this check on the draft states and add a non-draft test.

The PR objective requires validation to be skipped once the session is no longer draft.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/browser/views.py` around lines 172 - 188, Gate the
get_session_acroform_errors validation and its error response in the surrounding
endpoint so they run only when the session is in a draft state. Preserve the
existing blocking behavior for draft sessions, and add coverage confirming that
a non-draft session bypasses validation and proceeds normally.

@coveralls

coveralls commented Aug 12, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 31583480976

Coverage increased (+0.9%) to 86.886%

Details

  • Coverage increased (+0.9%) from the base build.
  • Patch coverage: 8 uncovered changes across 2 files (148 of 156 lines covered, 94.87%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/imio/esign/browser/views.py 33 28 84.85%
src/imio/esign/acroform.py 105 102 97.14%
Total (3 files) 156 148 94.87%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1487
Covered Lines: 1292
Line Coverage: 86.89%
Coverage Strength: 0.87 hits per line

💛 - Coveralls

@chris-adam
chris-adam force-pushed the PARAF-503/acroform-validation branch from 823161e to f0f5fc4 Compare August 12, 2026 08:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/imio/esign/tests/test_acroform.py (1)

36-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add DOCX extraction coverage.

extract_text() supports DOCX through word/document.xml, but this test covers only PDF and ODT. Add a minimal DOCX archive case that verifies both extract_text() and get_tag_ids().

The PR objective lists DOCX as a supported format.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/tests/test_acroform.py` around lines 36 - 55, Add a minimal
DOCX archive fixture in test_extract_text using word/document.xml, containing
representative text and a signer tag. Assert extract_text() returns the expected
normalized DOCX content and get_tag_ids() returns the corresponding tag IDs and
count, alongside the existing PDF and ODT coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/imio/esign/acroform.py`:
- Around line 151-153: Require a complete configured tag set whenever any
recognized signer or seal tag is present: update the validation flow around
get_tag_ids, validate_signer_numbers, and validate_seal_count so recognized tags
require every configured signer tag plus the configured seal tag, while
preserving acceptance of files with no tags. In src/imio/esign/acroform.py lines
151-153, implement this completeness check; in
src/imio/esign/tests/test_acroform.py lines 145-161, add regression cases for
seal-only files and signer tags missing the required seal tag.

---

Nitpick comments:
In `@src/imio/esign/tests/test_acroform.py`:
- Around line 36-55: Add a minimal DOCX archive fixture in test_extract_text
using word/document.xml, containing representative text and a signer tag. Assert
extract_text() returns the expected normalized DOCX content and get_tag_ids()
returns the corresponding tag IDs and count, alongside the existing PDF and ODT
coverage.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 909c56d3-c768-4e72-867c-006cdc487293

📥 Commits

Reviewing files that changed from the base of the PR and between 823161e and f0f5fc4.

📒 Files selected for processing (3)
  • src/imio/esign/acroform.py
  • src/imio/esign/browser/templates/acroform_errors.pt
  • src/imio/esign/tests/test_acroform.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/imio/esign/browser/templates/acroform_errors.pt

Comment on lines +151 to +153
numbers, seal_count = get_tag_ids(getattr(obj, "file", None))
errors = [] if nb_signers is None else validate_signer_numbers(numbers, nb_signers)
return errors + validate_seal_count(seal_count, seal)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Require a complete tag set when a recognized tag exists.

A session with two signers and a configured seal accepts a file containing only SCEAU. It also accepts signer tags when the configured seal tag is absent. This conflicts with the required rule of no tags or one tag for every configured signer and seal.

  • src/imio/esign/acroform.py#L151-L153: detect whether any recognized signer or seal tag exists. If it does, require all configured signer tags and the configured seal tag.
  • src/imio/esign/tests/test_acroform.py#L145-L161: add regression cases for a seal-only file with signers and signer tags without the required seal tag.
📍 Affects 2 files
  • src/imio/esign/acroform.py#L151-L153 (this comment)
  • src/imio/esign/tests/test_acroform.py#L145-L161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/imio/esign/acroform.py` around lines 151 - 153, Require a complete
configured tag set whenever any recognized signer or seal tag is present: update
the validation flow around get_tag_ids, validate_signer_numbers, and
validate_seal_count so recognized tags require every configured signer tag plus
the configured seal tag, while preserving acceptance of files with no tags. In
src/imio/esign/acroform.py lines 151-153, implement this completeness check; in
src/imio/esign/tests/test_acroform.py lines 145-161, add regression cases for
seal-only files and signer tags missing the required seal tag.

@chris-adam
chris-adam force-pushed the PARAF-503/acroform-validation branch from f0f5fc4 to 0000cb4 Compare August 12, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants